-
Notifications
You must be signed in to change notification settings - Fork 26
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Patch existing entities in POST /bundle/import
#1383
Conversation
ff76e29
to
2c33547
Compare
This reverts commit afc2391.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
That's a very sensitive PR, it would be nice to extend the QA section with the merge strategy for asset properties and dedicated tests to actively test the scenario behind this PR with score update.
"foouser" | ||
"foogroup" | ||
"user") | ||
(let [create-response (th/POST app |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟠 why did you need to use the http layer (+ authorization) instead of the existing bundle/import
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I couldn't figure out how to make the test run on my machine.
(if (empty? asset_refs) | ||
{} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess that's now it's already included in the first test of the loop, right?:
entities (loop [asset_refs asset_refs
entities {}]
(if (empty? asset_refs)
entities
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just left it in to skip a useless log.
Epic https://github.com/advthreat/iroh/issues/7341
Related https://github.com/advthreat/iroh/issues/8207
Current behavior of bundle import on existing entities is to ignore them. This PR changes behavior to patch in this case, and loosens the schema for bundle import so then partial entities are allowed if patching.
§ QA
This PR adds the ability for
POST /bundle/import
to patch entities. The following steps test if this procedure works.POST /bundle/import
. For example, patching the:producer
field of the indicator:"updated"
part is the most important part):GET /ctia/indicator
to verify that theproducer
field was updated.§ Release Notes
§ Squashed Commits